issue-2969: add hive reconnect time counter #2970

yegorskii · 2025-02-03T20:51:15Z

github-actions · 2025-02-03T23:03:24Z

Note

This is an automated comment that will be appended during run.

🟢 linux-x86_64-relwithdebinfo: all tests PASSED for commit d075141.

TESTS	PASSED	ERRORS	FAILED	SKIPPED	MUTED^?
5589	5589	0	0	0	0

SvartMetal · 2025-02-04T11:04:13Z

cloud/storage/core/libs/hive_proxy/hive_proxy_actor.cpp

+    } else if (msg->ClientId == HiveClientId && ReconnectStartTime)
+    {
+        if (ReconnectPipeCounter) {
+            ReconnectPipeCounter->Add(


этот счётчик показывает суммарное время всех реконнектов к Hive?

точно ли это полезная информация?

может считать среднее время реконнекта?

или перцентили

Реконнект всего один(hive-то один и pipe client по факту тоже только один) при разрыве. Мы откладываем время только после восстановления соединения. Не вижу смысла в гистограммах или перцентилях пока

Но по факту да, суммарное время реконнектов за 15 секунд

Нам это нужно что тыкать этим в YDB, когда hive слишком долго поднимается. Обсуждали с ними, такой метрики им достаточно

SvartMetal · 2025-02-04T11:04:52Z

cloud/storage/core/libs/hive_proxy/hive_proxy_actor.cpp

@@ -307,6 +340,12 @@ void THiveProxyActor::HandleConnectionError(

        for (const auto& actorId: states->Actors) {
            auto clientId = ClientCache->Prepare(ctx, hive);
+            if (!HiveClientId) {
+                HiveClientId = clientId;


зачем нужен HiveClientId?

Чтобы понять что текущий сдох после разрыва соединения. Судя по коду UnboundClientCache если клиент получает сообщение о разьединении то его просто убивают

SvartMetal · 2025-02-04T11:05:10Z

cloud/storage/core/libs/hive_proxy/hive_proxy_actor.h

@@ -177,14 +177,28 @@ class THiveProxyActor final

    const ui64 TenantHiveTabletId;

+    const NMonitoring::TDynamicCounterPtr CountersRoot;


просто Counters

SvartMetal · 2025-02-04T11:05:19Z

cloud/storage/core/libs/hive_proxy/hive_proxy_actor.h

@@ -177,14 +177,28 @@ class THiveProxyActor final

    const ui64 TenantHiveTabletId;

+    const NMonitoring::TDynamicCounterPtr CountersRoot;
+    NMonitoring::TDynamicCounters::TCounterPtr ReconnectPipeCounter;


HiveReconnectTimeCounter

SvartMetal · 2025-02-04T11:05:35Z

cloud/storage/core/libs/hive_proxy/hive_proxy_actor.h

@@ -177,14 +177,28 @@ class THiveProxyActor final

    const ui64 TenantHiveTabletId;

+    const NMonitoring::TDynamicCounterPtr CountersRoot;
+    NMonitoring::TDynamicCounters::TCounterPtr ReconnectPipeCounter;
+    ui64 ReconnectStartTime = 0;


HiveReconnectStartTime

SvartMetal · 2025-02-04T11:06:34Z

cloud/storage/core/libs/hive_proxy/hive_proxy_actor.cpp

@@ -216,6 +239,13 @@ void THiveProxyActor::HandleConnect(
        auto error = MakeKikimrError(msg->Status, TStringBuilder()
            << "Connect to hive " << hive << " failed");
        HandleConnectionError(ctx, error, hive, true);
+    } else if (msg->ClientId == HiveClientId && ReconnectStartTime)


зачем понадобился HiveClientId, почему не хватает проверки ReconnectStartTime?

Чтобы подстраховаться от возможных гонок

SvartMetal · 2025-02-04T11:07:26Z

cloud/storage/core/libs/hive_proxy/hive_proxy.h

 namespace NCloud::NStorage {

 ////////////////////////////////////////////////////////////////////////////////

 NActors::IActorPtr CreateHiveProxy(THiveProxyConfig config);
+NActors::IActorPtr CreateHiveProxy(
+    THiveProxyConfig config,
+    NMonitoring::TDynamicCounterPtr CountersRoot);


SvartMetal · 2025-02-04T11:08:02Z

cloud/storage/core/libs/hive_proxy/hive_proxy.cpp

+    if (config.FallbackMode) {
+        return std::make_unique<THiveProxyFallbackActor>(std::move(config));
+    }


копипаста

github-actions · 2025-02-04T14:05:07Z

Note

This is an automated comment that will be appended during run.

🟢 linux-x86_64-relwithdebinfo: all tests PASSED for commit 8baac1c.

TESTS	PASSED	ERRORS	FAILED	SKIPPED	MUTED^?
5589	5589	0	0	0	0

SvartMetal · 2025-02-04T18:47:37Z

cloud/storage/core/libs/hive_proxy/hive_proxy_actor.cpp

+            HiveReconnectTimeCounter->Add(
+                CyclesToDuration(GetCycleCount() - HiveReconnectStartTime).MicroSeconds());
+        }
+        HiveReconnectStartTime = 0;


HIveReconnectStartCycles

SvartMetal · 2025-02-04T18:49:28Z

cloud/storage/core/libs/hive_proxy/hive_proxy_ut.cpp

+        for (int retries = 0; retries < 5 && !hiveLockRequests; ++retries) {
+            // Pipe to hive may take a long time to connect
+            // Wait until hive receives the lock request
+            runtime.DispatchEvents(TDispatchOptions(), TDuration::Seconds(1));
+        }


лучше вечный цикл

github-actions · 2025-02-05T19:05:34Z

Note

This is an automated comment that will be appended during run.

🟢 linux-x86_64-relwithdebinfo: all tests PASSED for commit e1ff782.

TESTS	PASSED	ERRORS	FAILED	SKIPPED	MUTED^?
5589	5589	0	0	0	0

SvartMetal · 2025-02-05T19:08:15Z

cloud/storage/core/libs/hive_proxy/hive_proxy_actor.cpp

@@ -216,6 +237,14 @@ void THiveProxyActor::HandleConnect(
        auto error = MakeKikimrError(msg->Status, TStringBuilder()
            << "Connect to hive " << hive << " failed");
        HandleConnectionError(ctx, error, hive, true);
+    } else if (HiveReconnectStartCycles)
+    {


вроде не переносим скобку в этом случае

SvartMetal · 2025-02-05T19:08:44Z

cloud/storage/core/libs/hive_proxy/hive_proxy_actor.cpp

+    {
+        if (HiveReconnectTimeCounter) {
+            HiveReconnectTimeCounter->Add(
+                CyclesToDuration(GetCycleCount() - HiveReconnectStartCycles).MicroSeconds());


80 символов

SvartMetal · 2025-02-05T19:10:06Z

cloud/storage/core/libs/hive_proxy/hive_proxy_ut.cpp

+            CreateHiveProxy(
+                std::move(config), Runtime.GetAppData(0).Counters).release());


форматирование

issue-2969: add hive reconnect time counter

d075141

yegorskii added large-tests Launch large tests for PR blockstore Add this label to run only cloud/blockstore build and tests on PR filestore Add this label to run only cloud/filestore build and tests on PR labels Feb 3, 2025

SvartMetal reviewed Feb 4, 2025

View reviewed changes

update

8baac1c

SvartMetal reviewed Feb 4, 2025

View reviewed changes

update

e1ff782

SvartMetal reviewed Feb 5, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

issue-2969: add hive reconnect time counter #2970

issue-2969: add hive reconnect time counter #2970

yegorskii commented Feb 3, 2025

github-actions bot commented Feb 3, 2025

SvartMetal Feb 4, 2025

yegorskii Feb 4, 2025

yegorskii Feb 4, 2025

yegorskii Feb 4, 2025

SvartMetal Feb 4, 2025

yegorskii Feb 4, 2025

SvartMetal Feb 4, 2025

SvartMetal Feb 4, 2025

SvartMetal Feb 4, 2025

SvartMetal Feb 4, 2025

yegorskii Feb 4, 2025

SvartMetal Feb 4, 2025

SvartMetal Feb 4, 2025

github-actions bot commented Feb 4, 2025

SvartMetal Feb 4, 2025

SvartMetal Feb 4, 2025

github-actions bot commented Feb 5, 2025

SvartMetal Feb 5, 2025

SvartMetal Feb 5, 2025

SvartMetal Feb 5, 2025

		@@ -177,14 +177,28 @@ class THiveProxyActor final

		const ui64 TenantHiveTabletId;

		const NMonitoring::TDynamicCounterPtr CountersRoot;

		CreateHiveProxy(
		std::move(config), Runtime.GetAppData(0).Counters).release());

issue-2969: add hive reconnect time counter #2970

Are you sure you want to change the base?

issue-2969: add hive reconnect time counter #2970

Conversation

yegorskii commented Feb 3, 2025

github-actions bot commented Feb 3, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Feb 4, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Feb 5, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment